[Feature] Generic secondary resources #54

InDebt · 2025-05-03T13:51:45Z

Generic Secondary resources

This PR will enable generic secondary resources that can be configured for each individual Sim UI to reflect their respective resource.

Right now it can be used like this for example (note. the extra variable for ShadowOrb is just for taste and more expressivnes in the spell later)
in shadow.go

spriest.RegisterSecondaryResourceBar(core.SecondaryResourceConfig{
    Type: core.ShadowOrbs,
    Max:  3,
})

spriest.ShadowOrbs = spriest.SecondaryResourceBar

in mind_blast.go

shadow.ShadowOrbs.Gain(1, spell.ActionID, sim)

With a simple configuration like

secondaryResource: {
    color: "#b8a8f0",
    icon: "https://wow.zamimg.com/images/wow/icons/large/spell_priest_shadoworbs.jpg",
    name: "Shadow Orb",
}

The UI will display the generic resource accordingly.

InDebt · 2025-05-03T14:13:50Z

Looking into this - in theory we can add a new SecondaryResourceCost that would check for a certain secondary resource.
This resource has to be spent however, so spells that consume 'all' or 'UpTo' (i.E. 3 Holy Power) would either need a rework of the ApplyEffect method to supply the amount of resource used, or for those spells it needs to be done through the ExtraCondition and resources are spend in the ApplyEffect method.

This would be rather intransparent I feel. We can define a cost - but may only do so if the cost can not be spend before the ApplyEffect method. In my opinion it's easier and more transparent to simply define a ExtraCondition for all spells using a secondary resoruce and handle the appropiate spending logic within the spell.

Thoughts?

1337LutZ · 2025-05-03T15:13:19Z

This resource has to be spent however, so spells that consume 'all' or 'UpTo' (i.E. 3 Holy Power) would either need a rework of the ApplyEffect method to supply the amount of resource used, or for those spells it needs to be done through the ExtraCondition and resources are spend in the ApplyEffect method.

Yea I think I agree. Nerdegghead also mentioned it, as Spenders can have "variable" cost that can't be handled by SpellMods

ui/core/proto_utils/.action_id.ts.swp

NerdEgghead

I would also recommend ripping out the custom Holy Power resource implementation and replacing it with the new general framework as part of this PR. Now that we'll have the general version, we shouldn't keep any custom resource bars in the code anymore unless there's a good reason for it.

sim/core/resource_bar.go

InDebt · 2025-05-05T07:06:52Z

Cleaned some parts up based on feedback here. Also replaced the holy power implementation to be based on the SecondaryResourceBar. Due to Divine Purpose I actually just implemented a light weight HolyPowerBar that implements the Divine Purpose logic.

After touching the Holy Power implementation though, I guess we also need to provider proper APL support.

…s/mop into feature/generic-resources

InDebt · 2025-05-05T13:19:01Z

With the Help of @1337LutZ APL support has been integrated now as well that dynamically resolves the resource name for the APL action.

NerdEgghead

Getting closer!

proto/common.proto

proto/spell.proto

sim/core/apl_values_resources.go

sim/core/resource_bar.go

sim/paladin/holy_power_bar.go

sim/core/resource_bar.go

sim/paladin/paladin.go

hillerstorm · 2025-05-06T06:26:37Z

Cleaned some parts up based on feedback here. Also replaced the holy power implementation to be based on the SecondaryResourceBar. Due to Divine Purpose I actually just implemented a light weight HolyPowerBar that implements the Divine Purpose logic.

After touching the Holy Power implementation though, I guess we also need to provider proper APL support.

In my local pala branch I've actually cleaned up the DP proc logic a bit by having the holy power bar have a pub/sub for resource gain/spent, I can just do that on top of these changes later 🙂 seems like a useful thing

hillerstorm · 2025-05-06T06:29:46Z

Cleaned some parts up based on feedback here. Also replaced the holy power implementation to be based on the SecondaryResourceBar. Due to Divine Purpose I actually just implemented a light weight HolyPowerBar that implements the Divine Purpose logic.

After touching the Holy Power implementation though, I guess we also need to provider proper APL support.

In my local pala branch I've actually cleaned up the DP proc logic a bit by having the holy power bar have a pub/sub for resource gain/spent, I can just do that on top of these changes later 🙂 seems like a useful thing

And now I see you also added the pub/sub 😅 nice!

… the logs

…ources

InDebt · 2025-05-06T11:06:40Z

Alright - Chi is gone

NerdEgghead

Looks great, just added one additional comment.

sim/core/energy.go

[Feature] Generic secondary resouces

bbf50c3

InDebt requested a review from 1337LutZ May 3, 2025 13:51

github-actions bot added the Priest label May 3, 2025

1337LutZ requested a review from NerdEgghead May 3, 2025 18:54

Merge branch 'master' into feature/generic-resources

2e79f66

1337LutZ reviewed May 4, 2025

View reviewed changes

ui/core/proto_utils/.action_id.ts.swp Outdated Show resolved Hide resolved

Remove unintended file

837193f

InDebt marked this pull request as ready for review May 4, 2025 16:44

NerdEgghead requested changes May 5, 2025

View reviewed changes

[Feature]Generic Resoruces: Update implementation and replace Holy Power

eda54c5

github-actions bot added the Paladin label May 5, 2025

[Feature] Generic Resources: Remove line comments from test

5d1fe07

Marcel Ellermann and others added 6 commits May 5, 2025 09:10

[Feature] Generic Resource: Remove old proto mappings

98dd829

[Feature] Generic resources - Part 1 APL implementation

ddea63f

[Feature] Generic Resource: Add support for callbacks

758fb68

[Feature] Generic Resource: Add missing callback invoke

c49ff3c

Add SecondaryResource class & extend APL dynamic string

5d8d680

Merge branch 'feature/generic-resources' of https://github.com/wowsim…

1bd49b8

…s/mop into feature/generic-resources

github-actions bot removed the Priest label May 5, 2025

[Feature] Generic Resource: Fix APL

d53ff8a

[Feature] Generic Resources: Make bar track Int32

b1592a1

NerdEgghead requested changes May 6, 2025

View reviewed changes

Marcel Ellermann added 2 commits May 6, 2025 09:01

[Feature] Generic Resource: Update resource implementation

0022a93

[Feature] Generic Resources: Make resource bar private to core

c4c9ecf

Marcel Ellermann added 3 commits May 6, 2025 09:34

[Feature] Generic Resource: Map secondary resource names correctly in…

014be3c

… the logs

Merge remote-tracking branch 'origin/master' into feature/generic-res…

e1ed415

…ources

[Feature] Generic resource: Remove chi from secondary resources

5ac1cb8

NerdEgghead approved these changes May 6, 2025

View reviewed changes

sim/core/energy.go Outdated Show resolved Hide resolved

Marcel Ellermann added 2 commits May 9, 2025 13:16

Merge branch 'master' into feature/generic-resources

44bce62

[Feature] Generic Resouce: Fixup unintended removed code

54c93d8

InDebt merged commit e0ccf6a into master May 9, 2025
2 checks passed

1337LutZ deleted the feature/generic-resources branch December 6, 2025 12:32

[Feature] Generic secondary resources #54

[Feature] Generic secondary resources #54

Uh oh!

Conversation

InDebt commented May 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Generic Secondary resources

Uh oh!

InDebt commented May 3, 2025

Uh oh!

1337LutZ commented May 3, 2025

Uh oh!

Uh oh!

NerdEgghead left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

InDebt commented May 5, 2025

Uh oh!

InDebt commented May 5, 2025

Uh oh!

NerdEgghead left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hillerstorm commented May 6, 2025

Uh oh!

hillerstorm commented May 6, 2025

Uh oh!

InDebt commented May 6, 2025

Uh oh!

NerdEgghead left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

InDebt commented May 3, 2025 •

edited

Loading